Skip to content

Add check for consistency in land cover and soil - #100

Merged
GreyREvenson merged 12 commits into
NOAA-OWP:noah_om_gridfrom
GreyREvenson:noah_om_grid_issue97
May 22, 2024
Merged

Add check for consistency in land cover and soil#100
GreyREvenson merged 12 commits into
NOAA-OWP:noah_om_gridfrom
GreyREvenson:noah_om_grid_issue97

Conversation

@GreyREvenson

@GreyREvenson GreyREvenson commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

Purpose

This PR fixes issue #97.

Additions

Added land cover and soil consistency checks to initialize_from_file in RunModule.f90. These checks throw an error and stop the program if any cell is initialized with the following conditions:

  • land use type != water and soil type = water
  • land use type = water and soil type != water
  • land use type != ice and soil type = ice
  • land use type = ice and soil type != ice

Testing

I manually/hardcoded problematic values for vegtyp and isltyp to produce errors:

where (domaingrid%mask.eq.1) domaingrid%vegtyp = parametersgrid%ISWATER
where (domaingrid%mask.eq.1) domaingrid%isltyp = 1
returns => ERROR: one or more grid cells have a water land cover (vegtyp = 16) and a non-water soil type (isltyp != 14)

where (domaingrid%mask.eq.1) domaingrid%vegtyp = 1
where (domaingrid%mask.eq.1) domaingrid%isltyp = 14
returns => ERROR: one or more grid cells have a non-water land cover (vegtyp != 16) and a water soil type (isltyp = 14)

where (domaingrid%mask.eq.1) domaingrid%vegtyp = parametersgrid%ISICE
where (domaingrid%mask.eq.1) domaingrid%isltyp = 1
returns => ERROR: one or more grid cells have an ice land cover (vegtyp = 24) and a non-ice soil type (isltyp != 16)

where (domaingrid%mask.eq.1) domaingrid%vegtyp = 1
where (domaingrid%mask.eq.1) domaingrid%isltyp = 16
returns => ERROR: one or more grid cells have a non-ice land cover (vegtyp != 24) and an ice soil type (isltyp = 16)

Notes

  • Implemented checks using where statements to avoid iterating over the grid
  • Checks were implemented with hardcoded water and ice values for soil type. To avoid using hardcoded soil type values, we could require that the user provide ISWATER and ISICE values for the utilized soil classification scheme (as the user already must provide for the utilized land cover classification scheme).

@GreyREvenson
GreyREvenson marked this pull request as draft February 22, 2024 15:55
@GreyREvenson GreyREvenson linked an issue Feb 22, 2024 that may be closed by this pull request
@GreyREvenson GreyREvenson changed the title Adding check for consistency in land cover and soil Add check for consistency in land cover and soil Feb 26, 2024
@GreyREvenson
GreyREvenson marked this pull request as ready for review March 18, 2024 15:37
@GreyREvenson
GreyREvenson requested a review from drakest123 March 18, 2024 15:38
@drakest123

Copy link
Copy Markdown

This PR has sanity checks to ensure grid cells that define water or ice do not also have a vegetation type. The sanity checks touched only the initialize_from_file subroutine in RunModule.f90.

  • Compiled and executed: run/noah_owp_modular.exe
  • test/noahowp_test.exe ../run/namelist.input > output_test
  • No errors were detected.
  • Sanity tests were coded.
  • Checks were optimized by using the “where” keyword.
  • The algorithm makes use of an array, err_grid(:,:). This may be a target for optimization if memory usage becomes an issue.
  • It might be convenient for the user to report problematic grid cell locations rather than the fact that there is/are problematic grid cells.

Comment thread src/RunModule.f90 Outdated

@drakest123 drakest123 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, report problematic grid cell locations rather than the fact that there is/are problematic grid cells.

@GreyREvenson

Copy link
Copy Markdown
Contributor Author

Thanks for the review @drakest123. I went ahead and add locational information to the error messages.

@GreyREvenson

Copy link
Copy Markdown
Contributor Author

@SnowHydrology Do you think this PR's checks are too restrictive? Here, we're requiring grid cells that have ice land cover to have ice soils as well.

@SnowHydrology SnowHydrology left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good with one exception: using stop prevents the framework from returning useful error information. Instead, I'd recommend using return and then having the initialize routine return BMI_FAILURE. See examples here and here.

@GreyREvenson

Copy link
Copy Markdown
Contributor Author

Note: I'm leaving the stop statements as-is with the assumption that the issue will be corrected by PR #108.

Comment thread src/RunModule.f90
integer :: forcing_timestep ! integer time step (set to dt) for some subroutine calls
integer :: ii ! indices
integer, allocatable, dimension(:,:) :: err_grid ! to catch land use vs. soil inconsistencies
integer, dimension(2) :: err_indices ! to hold ix, iy for err_grid

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: it may be preferable to move these error-related objects to the ErrorCheckModule if PR #108 elects to make greater use of that module for error checking and messaging purposes.

@GreyREvenson
GreyREvenson merged commit b157538 into NOAA-OWP:noah_om_grid May 22, 2024
@GreyREvenson
GreyREvenson deleted the noah_om_grid_issue97 branch May 22, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check for consistency in land cover and soil

4 participants